Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix dependency on redis-cli and redis-server to pass tests #3014

Merged
merged 7 commits into from
Feb 10, 2025
Merged

Fix dependency on redis-cli and redis-server to pass tests #3014

merged 7 commits into from
Feb 10, 2025

Conversation

avifenesh
Copy link
Member

Trying to work with Fedora which has strict rules about not OSS packages.
The Redis symlink doesn't exist in the distro because of it. When running tests, they fail, since there are many places we depend on the symlinks to exist.

This pr decouple and allow to developer to have just valkey on their system.

This pr also introduce the way to download Valkey using the package managers of the different distros, since those are not explained yet on Valkey sites.

Issue link

This Pull Request is linked to issue #3011

Checklist

Before submitting the PR make sure the following are checked:

  • This Pull Request is related to one issue.
  • Commit message has a detailed description of what changed and why.
  • Tests are added or updated.
  • CHANGELOG.md and documentation files are updated.
  • Destination branch is correct - main or release
  • Create merge commit if merging release branch into main, squash otherwise.

@avifenesh avifenesh requested a review from a team as a code owner January 26, 2025 15:14
@avifenesh avifenesh linked an issue Jan 26, 2025 that may be closed by this pull request
Tests fails if having valkey downloaded from package managers of distros with strict OSS rules #3011
Closed
@Yury-Fridlyand Yury-Fridlyand added the node Node.js wrapper label Jan 27, 2025
@Yury-Fridlyand
Copy link
Collaborator

I propose to split this into 3 PRs, because it contains independent features - doc update, node client fixes and core fixes

@avifenesh
Copy link
Member Author

avifenesh commented Jan 27, 2025 •
edited
Loading

I propose to split this into 3 PRs, because it contains independent features - doc update, node client fixes and core fixes

I'll split into two, doc and tests. The core one is just one change and naming, not sure if it needs a PR for itself.
It actually was a small fix, since I changed to fedora for a bit, discovered that yum already has it and that we are breaking on the valkey installed on fedora because of the strict rules. Then I thought, well, if fedora has it in yum, probably there are others. And apparently all the main ones does so, added to docs.

BTW, on top of mac, Ubuntu and for node, also alpine, glide is compatible with Fedora and Mint, I checked it, and most likely all the CentOS/RHEL family and all the successors of Debian.
Just Arch is in question.

@avifenesh avifenesh force-pushed the valkey-cli-as-option branch from f8d8447 to f05ddc4 Compare February 5, 2025 16:24
@avifenesh avifenesh force-pushed the valkey-cli-as-option branch from 4ad2ac5 to f7e0ef3 Compare February 5, 2025 16:55
@avifenesh avifenesh force-pushed the valkey-cli-as-option branch from f7e0ef3 to 6d39dc3 Compare February 5, 2025 16:56
shohamazon
shohamazon approved these changes Feb 10, 2025
View reviewed changes
eslint.config.mjs Show resolved Hide resolved
glide-core/redis-rs/redis/tests/support/mod.rs Outdated Show resolved Hide resolved
node/hybrid-node-tests/commonjs-test/package.json Show resolved Hide resolved
node/npm/glide/package.json Show resolved Hide resolved
avifenesh and others added 6 commits February 10, 2025 18:44
@avifenesh
Update developer documentation to reflect Valkey branding and install…
ccd56c1 
…ation instructions

Signed-off-by: avifenesh <[email protected]>
@avifenesh
Update dependencies and improve error handling in Redis and cluster m…
d511682 
…anagement

Signed-off-by: avifenesh <[email protected]>
@avifenesh
Refactor package.json to use dynamic naming and update development de…
e3b78be 
…pendencies

Signed-off-by: avifenesh <[email protected]>
@avifenesh
Refactor ESLint configuration and improve test code formatting
3d97d06 
Signed-off-by: avifenesh <[email protected]>
@avifenesh
Update package.json for valkey-glide: rename package, upgrade devDepe…
bc73900 
…ndencies, and update long dependency version

Signed-off-by: avifenesh <[email protected]>
@avifenesh @shohamazon
Update glide-core/redis-rs/redis/tests/support/mod.rs
b8c2853 
Co-authored-by: Shoham Elias <[email protected]>
Signed-off-by: Avi Fenesh <[email protected]>
@avifenesh avifenesh force-pushed the valkey-cli-as-option branch from 63f68f3 to b8c2853 Compare February 10, 2025 18:44
@avifenesh avifenesh merged commit ad9774d into main Feb 10, 2025
64 checks passed
@avifenesh avifenesh deleted the valkey-cli-as-option branch February 10, 2025 23:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@shohamazon shohamazon shohamazon approved these changes

@jhpung jhpung Awaiting requested review from jhpung

@Muhammad-awawdi-amazon Muhammad-awawdi-amazon Awaiting requested review from Muhammad-awawdi-amazon

Assignees
No one assigned
Labels
bug Something isn't working node Node.js wrapper testing Everything about testing
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Tests fails if having valkey downloaded from package managers of distros with strict OSS rules
3 participants
@avifenesh @Yury-Fridlyand @shohamazon